Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding <thead> to tables #387

Merged
merged 8 commits into from Mar 24, 2014
Merged

Adding <thead> to tables #387

merged 8 commits into from Mar 24, 2014

Conversation

Klap-in
Copy link
Collaborator

@Klap-in Klap-in commented Oct 17, 2013

Wrap thead around 1st row, when 1st cell at 1st row is tableheader.

Implements the feature request FS#1764

@splitbrain
Copy link
Collaborator

That makes no sense for a table like this:

^ Weight: | 55kg |
^ Height: | 20cm |
^ Length: | 15cm |

A thead would only make sense when all cells in the first row are headers.

@Klap-in
Copy link
Collaborator Author

Klap-in commented Oct 18, 2013

3 questions:

  1. Is there a neat way to count forward, or is that simple looping over the list of calls until i count tableheader/tablecell calls?
  2. Should i add <tbody> too? (see reply of @selfthinker https://bugs.dokuwiki.org/index.php?do=details&task_id=1764#comment5456)
  3. In html are only open tags fine for rowgroups. Are only open tags allowed/sensible in the instructions too?, or is it better to have always open and close combinations in DokuWiki instructions? http://www.w3.org/TR/html401/struct/tables.html#h-11.2.3

@Klap-in
Copy link
Collaborator Author

Klap-in commented Oct 18, 2013

about 1. : Of course i can add a counter in the existing for loop, but that i need to add a element before the current <rw> element when i'm at the </rw>, is looping backward a wanted solution?

@splitbrain
Copy link
Collaborator

  1. no there's no elegant way to do that I guess. You could remember the position of the opening row and then handle the thead when the row is closed, inserting to the remembered position. That would avoid too much looping
  2. maybe- maybe not. thead and tbody are semantic elements which are really hard to autogenerate because we know nothing about the semantics of the table in question
  3. no, please avoid unclosed elements, we try to be polyglot and be valid xhtml and html5 so no open tags allowed.

@Chris--S
Copy link
Collaborator

Thead should not only apply to the first row, but all contiguous rows at the top of the table where every cell is a <th>

@michitux
Copy link
Collaborator

Wouldn't it be possible to store flags in the handler instance when the th elements are collected (i.e. before the processing step) that track in which row the first non-th table element occurred?

@Chris--S
Copy link
Collaborator

There are two processing steps in the table call rewriter, process() & then tablefinalize() when process() hits tableend.

Can you add a inHead flag and countHeadRows to the class. In process(), start with inHead true and set it to false when you hit the first tablecell. In rowclose increment countHeadRows if inHead is still true. Then when you get to finalizetable(), if countHeadRows is not zero put in your tableheadopen along with the tableopen and then put in the tableheadclose when you get to the nth (n = countHeadRows) tablerowclose.

[ I'm not sure how to detect an unclosed rowspan, so i think perhaps you also set inHead flag to false if you hit any rowspan ]

@Chris--S
Copy link
Collaborator

Klap-in, I've put my changes into the branch tablethead_chris and created a #390 for them to this branch. Please merge the PR if you are ok with the changes.

@selfthinker
Copy link
Collaborator

A thead would only make sense when all cells in the first row are headers.

I would disagree with that. A thead could have mostly th's, but it would be absolutely fine to have a few td's. One example I could think of is when the first cell is empty.

I would always add a tbody. In normal tables it should be around everything. In tables where you add a thead, it should be around the rest.

@Chris--S
Copy link
Collaborator

@selfthinker - re, tds in thead, can you give me an example of what you mean in dokuwiki syntax.

@selfthinker
Copy link
Collaborator

E.g. the second table on here: https://www.dokuwiki.org/wiki:syntax#tables

@Chris--S
Copy link
Collaborator

We'll need some definitive rules to test...how about?

  • less than 2 TD in a row, &
  • TD have to be less than 50% of total cells

@splitbrain
Copy link
Collaborator

@Chris--S These rules sound good to me.

One addition: the row should have more than one cell.

@selfthinker
Copy link
Collaborator

I disagree with "the row should have more than one cell". I can think of perfectly valid examples with just one column, where the first one is the th and the rest are tds.

@splitbrain
Copy link
Collaborator

right, I guess the rule should be "should have more than one row" then

@selfthinker
Copy link
Collaborator

Yes, that makes sense.

@selfthinker
Copy link
Collaborator

What happened to this one? Only refining when to add theads according to the agreed rules is left, right?

@splitbrain
Copy link
Collaborator

@Chris--S @Klap-in can you guys finish this one up and merge it?

@Chris--S
Copy link
Collaborator

@splitbrain @selfthinker can you clarify the comments from "One addition ..." to "... Yes, that makes sense".

I don't see what effect the number of rows has. There can be one cell in a header row, but only if its a TH. That is allowed by my original two rules - they had an unstated assumption, when the rest of the cells are TH.

@Chris--S
Copy link
Collaborator

In addition to above. How to treat these tables:

^ KEY | value |

^ HEADING | stuff ^

For me, first one does not get THEAD. And the second one does - by putting the '^' at the end of the row.

@selfthinker
Copy link
Collaborator

@splitbrain @selfthinker can you clarify the comments from "One addition ..." to "... Yes, that makes sense".
I don't see what effect the number of rows has.

If there is only one row, there shouldn't be a thead.

In addition to above. How to treat these tables:
^ KEY | value |
^ HEADING | stuff ^

The second is invalid syntax, so there won't be a good solution. Therefore it doesn't really matter what you do. ;-)

@Chris--S
Copy link
Collaborator

Ok. so rule 3. table must have more than one row.
And I guess 3a. not all table rows can be thead rows.

The second syntax works perfectly well - https://www.dokuwiki.org/sandbox_table

@splitbrain
Copy link
Collaborator

Yeah it's valid syntax. Up til now the last closing row char did not really matter and was usually the same as the opening char of the same cell, but there's no rule for that. I would like to avoid giving it meaning now. The leading char defines the cell type and we should only use that for deciding on theads IMHO.

@selfthinker
Copy link
Collaborator

Ok. so rule 3. table must have more than one row.
And I guess 3a. not all table rows can be thead rows.

If you have 3, 3a is redundant. Or the other way around. Just to make this clear: tbody always makes sense, even in tables with just one row.

Yeah it's valid syntax.

AFAIK it is not. wiki:syntax says:

it's the cell separator before a cell which decides about the formatting

That it works and doesn't break in the example above has nothing to do with validity.

@Chris--S
Copy link
Collaborator

Its the other way around, 3a makes 3 rendundant. Consider:

^ r1c1 ^ r2c2 ^
^ r2c1 ^ r2c2 ^

Which I guess should result in a table containing all <TH> cells but no <THEAD> rows.

@selfthinker
Copy link
Collaborator

Hmm, yeah, okay.

…element

1. TD < 2 in a row
2. TD <= 50% of total cells
3. Not all table rows can be THEAD rows
@Chris--S
Copy link
Collaborator

Here are some sample tables, https://gist.github.com/Chris--S/9589178

This branch currently renders THEAD as described in the text.

@splitbrain
Copy link
Collaborator

The samples look good to me, but the branch breaks a bunch of tests right now.

@splitbrain
Copy link
Collaborator

👍

splitbrain added a commit that referenced this pull request Mar 24, 2014
@splitbrain splitbrain merged commit 4afeda5 into master Mar 24, 2014
@splitbrain splitbrain deleted the tablethead branch March 24, 2014 10:37
@Klap-in
Copy link
Collaborator Author

Klap-in commented Mar 24, 2014

Is it useful to mention the thead feature of the tables at the wiki:syntax page? Or is it too technically, so too much buzz for a common wiki user?

@splitbrain
Copy link
Collaborator

yeah, too much buzz for the end user I'd say

@selfthinker
Copy link
Collaborator

I agree, syntax-wise it's not important and it's difficult to explain to non-technical people. It wouldn't even change the visible output (unless we, or a different template, added different styling for theads).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants